Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

[WASM] mem_cmp added to the Wasm runtime#7539

Merged
NikVolf merged 4 commits into
masterfrom
mem_cmp
Jan 15, 2018
Merged

[WASM] mem_cmp added to the Wasm runtime#7539
NikVolf merged 4 commits into
masterfrom
mem_cmp

Conversation

@lexfrl
Copy link
Copy Markdown
Contributor

@lexfrl lexfrl commented Jan 11, 2018

In order to support wasm-unknown-unknown target runtime has to provide memcmp which previously was provided by emscripten

@lexfrl lexfrl added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jan 11, 2018
Comment thread ethcore/wasm/src/runtime.rs Outdated
let cx = context.value_stack.pop_as::<i32>()? as u32;
let ct = context.value_stack.pop_as::<i32>()? as u32;

self.charge(|schedule| schedule.wasm.mem_copy as u64 * len as u64)?; // TODO: charge cmp
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo should be addressed

@NikVolf NikVolf added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 11, 2018
@NikVolf NikVolf added A8-looksgood 🦄 Pull request is reviewed well. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Jan 12, 2018
let cx = self.memory.get(cx, len as usize)?;
let ct = self.memory.get(ct, len as usize)?;
let result = unsafe {
memcmp(cx.as_ptr() as *const c_void, ct.as_ptr() as *const c_void, len as usize)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memcmp should fail when invoked on overlapping region, because otherwise it's undefined behaviour?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, on the second thought, it shouldn't be

@NikVolf NikVolf added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. A8-looksgood 🦄 Pull request is reviewed well. and removed A8-looksgood 🦄 Pull request is reviewed well. A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jan 12, 2018
Comment thread ethcore/wasm/src/runtime.rs Outdated
{
//
// method signature:
// fn memcmp(cx: *const c_void, ct: *const c_void, n: usize) -> i32;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NikVolf NikVolf merged commit d927320 into master Jan 15, 2018
@NikVolf NikVolf deleted the mem_cmp branch January 15, 2018 14:24
@5chdn 5chdn added this to the 1.10 milestone Jan 16, 2018
tomusdrw pushed a commit that referenced this pull request Feb 14, 2018
* mem_cmp added to the Wasm runtime

* schedule.wasm.mem_copy to schedule.wasm.mem_cmp for mem_cmp
5chdn pushed a commit that referenced this pull request Feb 14, 2018
* update back-references more aggressively after answering from cache (#7578)

* Add new EF ropstens nodes. (#7824)

* Add new EF ropstens nodes.

* Fix tests

* Add a timeout for light client sync requests (#7848)

* Add a timeout for light client sync requests

* Adjusting timeout to number of headers

* Flush keyfiles. Resolves #7632 (#7868)

* Fix wallet import (#7873)

* rpc: generate new account id for imported wallets

* ethstore: handle duplicate wallet filenames

* ethstore: simplify deduplication of wallet file names

* ethstore: do not dedup wallet filenames on update

* ethstore: fix minor grumbles

* [WASM] mem_cmp added to the Wasm runtime (#7539)

* mem_cmp added to the Wasm runtime

* schedule.wasm.mem_copy to schedule.wasm.mem_cmp for mem_cmp

* [Wasm] memcmp fix and test added (#7590)

* [Wasm] memcmp fix and test added

* [Wasm] use reqrep_test! macro for memcmp test

* wasmi interpreter (#7796)

* adjust storage update evm-style (#7812)

* disable internal memory (#7842)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants